[DNM][env_op_images] Add unit tests for verify_pulled_report_crio#3914
[DNM][env_op_images] Add unit tests for verify_pulled_report_crio#3914nemarjan wants to merge 1 commit into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
recheck |
1 similar comment
|
recheck |
nemarjan
changed the title
nemarjan
force-pushed
the
verify-pulled-report-unit-test
branch
from
ad595db to
b32c182
Compare
evallesp
left a comment
evallesp
left a comment
There was a problem hiding this comment.
LGTM in general, some questions.
| "image_id": "quay.io/demo/other@sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", | ||
| "node": "node-b", | ||
| }, | ||
| {"image_id": "no-digest-here", "node": "node-a"}, |
There was a problem hiding this comment.
(non-blocking) question: Do we need this? I'm unsure if we're using somewhere.
There was a problem hiding this comment.
Yes, this entry is needed as it exercises the skip branch in run_module() where images without a sha256: digest are ignored. In practice, digest-less images can appear when containers are referenced by tag only (e.g. quay.io/demo/app:latest) and the report captures the image_id before CRI-O resolves it, or when images are locally built/imported without a registry digest. The module can't match these against CRI-O pull logs, so it skips them. This test entry ensures the skip works correctly and that entries_with_digest only counts images that actually carry a digest (2, not 3).
| with self.assertRaises(AnsibleFailJson) as rst: | ||
| VERIFY_MODULE.run_module() | ||
|
|
||
| self.assertIn("No CRI-O log files", rst.exception.args[0]["msg"]) |
There was a problem hiding this comment.
(blocking) question:I guess we can have more tests case for the exception at L211, L215, L175
There was a problem hiding this comment.
Addressed below , each of these exception paths now has a dedicated test:
L175 (IOError in log reading) → test_fails_when_log_file_unreadable
L211 (IOError in report reading) → test_fails_when_report_unreadable
L215 (YAMLError in parsing) → test_fails_when_report_has_invalid_yaml"
nemarjan
changed the title
nemarjan
force-pushed
the
verify-pulled-report-unit-test
branch
from
b32c182 to
2371b09
Compare
nemarjan
force-pushed
the
verify-pulled-report-unit-test
branch
2 times, most recently
from
f29c7d2 to
cdc44cd
Compare
|
recheck |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 11m 28s |
Add focused unit coverage for CRI-O pulled report verification, including successful enrichment, cross-node evidence accounting, and failure when no logs are provided. Reuse shared test utilities with a local fallback so tests run in both collection-style and local environments. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: nemarjan <nemarjan@redhat.com>
nemarjan
force-pushed
the
verify-pulled-report-unit-test
branch
from
cdc44cd to
ef38a7e
Compare
2 participants
Add focused unit coverage for CRI-O pulled report verification, including successful enrichment, cross-node evidence accounting, and failure when no logs are provided. Reuse shared test utilities with a local fallback so tests run in both collection-style and local environments.
Assisted-By: Cursor Code